Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: custom endpoint validation [IDE-126] #454

Merged
merged 3 commits into from
May 2, 2024

Conversation

teodora-sandu
Copy link
Contributor

@teodora-sandu teodora-sandu commented Apr 29, 2024

Description

Improves the validation so that only http(s)://api. APIs are configured in the custom endpoint field.

Tested manually by running the extension with the latest CLI.

Checklist

  • Tests added and all succeed
  • Linted
  • CHANGELOG.md updated
  • README.md updated, if user-facing

Screenshots / GIFs

Screenshot 2024-05-01 at 09 32 31 Screenshot 2024-05-01 at 09 32 26

@teodora-sandu teodora-sandu force-pushed the fix/custom-endpoint-validation branch from 9a37230 to 45451b2 Compare April 29, 2024 16:47
@teodora-sandu teodora-sandu force-pushed the fix/custom-endpoint-validation branch from 45451b2 to 25f525e Compare May 1, 2024 08:33
package.json Outdated
"markdownDescription": "Sets API endpoint to use for Snyk requests. Useful for custom Snyk setups. E.g. `https://api.eu.snyk.io`.",
"scope": "window",
"format": "uri",
"pattern": "^(https?://)api.?[a-zA-Z0-9]{0,19}.(snyk|snykgov).io$"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing to call out here is that I noticed in IntelliJ the custom endpoint supports http(s)://snyk.io/... endpoints. VSCode never did though and I'm not sure why

@teodora-sandu teodora-sandu marked this pull request as ready for review May 1, 2024 08:43
@teodora-sandu teodora-sandu requested a review from a team as a code owner May 1, 2024 08:43
@@ -204,13 +199,6 @@ export class Configuration implements IConfiguration {
return `${hostnameParts[2]}.${hostnameParts[3]}`.includes('snykgov.io');
}

get analyticsPermitted(): boolean {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code deleted in this file doesn't seem to be used anywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added in 8c2811b but the itly analytics have been removed since

@@ -292,10 +280,6 @@ export class Configuration implements IConfiguration {
return Configuration.source;
}

get baseApiUrl(): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added in 213c781 but it seems the usage has since been removed

Copy link
Contributor

@cat2608 cat2608 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just need to allow empty input too and everything is working fine!
🚀

@teodora-sandu
Copy link
Contributor Author

Allowing empty input too now:
Screenshot 2024-05-01 at 16 55 46

@teodora-sandu teodora-sandu merged commit 58bd8ae into main May 2, 2024
4 checks passed
@teodora-sandu teodora-sandu deleted the fix/custom-endpoint-validation branch May 2, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants